Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setup buildkite as an alternative to AZP #4771

Merged
merged 1 commit into from
Jan 17, 2023
Merged

Setup buildkite as an alternative to AZP #4771

merged 1 commit into from
Jan 17, 2023

Conversation

vchuravy
Copy link
Member

@vchuravy vchuravy commented Apr 16, 2022

TODO:

  • Nail down artifact upload story
    • Using buildkite upload is neat since we get something in the UI
    • Ideally we should upload directly to our own buckets, but need to make sure not to leak keys
      • Should be good citizens and not cause tremendous traffic to BK buckets
      • Can we generate ephermal keys? We could inject those from the generator script
  • Test jll_init and register steps
  • CCACHE/distcc
  • Limit concurreny of registration
  • @giordano hook up remarks from binary verifier to BK annotations (maybe a custom logger?)
  • Party like it's the 90s
  • Double check that ccache is actually working

.buildkite/generator.jl Outdated Show resolved Hide resolved
@vchuravy
Copy link
Member Author

Ah great we can also do logs extraction to create warnings https://buildkite.com/docs/agent/v3/cli-annotate

@vchuravy vchuravy force-pushed the bk/initial branch 3 times, most recently from 81e570d to 9c10f73 Compare April 17, 2022 11:35
@giordano
Copy link
Member

Some comments about what we currently do in Azure Pipelines. In terms of race conditions:

  • the JLL repo init job is not safe: in some cases we can build multiple versions of the same package (when they're split in different directories, e.g. CUDA, OpenBLAS or LLVM)
  • the build and package registration jobs are safe, however in some cases registration may cause merge conflicts in the registry (but we have relatively little control over that). Note that currently we do registrations in sequence (Don't allow parallel registration #208)

Note that buildkite is currently running, and failing, also on master: https://buildkite.com/julialang/yggdrasil/builds/4#d85857dd-92f6-43cd-82b5-2827d84e5602. It'd be nice to disable it until this is working

@vchuravy
Copy link
Member Author

It'd be nice to disable it until this is working

Done.

@giordano giordano added the meta 🌳 Issues and pull requested related to Yggdrasil itself label Apr 18, 2022
@DilumAluthge DilumAluthge added the ci continuous integration label Apr 18, 2022
.buildkite/generator.jl Outdated Show resolved Hide resolved
Copy link
Member

@giordano giordano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First round of comments.

Reminder that skipping build isn't implemented at the moment.

.buildkite/generator.jl Outdated Show resolved Hide resolved
.buildkite/generator.jl Outdated Show resolved Hide resolved
.buildkite/generator.jl Outdated Show resolved Hide resolved
.buildkite/generator.jl Show resolved Hide resolved
.buildkite/generator.jl Outdated Show resolved Hide resolved
.buildkite/generator.jl Outdated Show resolved Hide resolved
.buildkite/generator.jl Outdated Show resolved Hide resolved
.buildkite/generator.jl Outdated Show resolved Hide resolved
.buildkite/generator.jl Outdated Show resolved Hide resolved
.buildkite/pipeline.yml Outdated Show resolved Hide resolved
@vchuravy
Copy link
Member Author

So the current failure is real and I don't know why and how.
ERROR: LoadError: failed process: Process(git merge-base --fork-point remotes/origin/master HEAD, ProcessExited(1)) [1]

It seems to occur always when master is ahead of this feature branch, but git does not give a useful error.

@giordano
Copy link
Member

I guess that's because this doesn't run on the merge commit (which is my gripe with buildkite) 🙃

@DilumAluthge
Copy link
Member

When I have some free time, I want to get https://github.com/JuliaCI/pull-request-merge-commit-buildkite-plugin working. Then you would just stick that plugin at the top of your list of plugins, and it would automatically take care of checking out the merge commit.

Don't use it yet though, it doesn't work.

.buildkite/generator.jl Outdated Show resolved Hide resolved
@vchuravy
Copy link
Member Author

I guess that's because this doesn't run on the merge commit (which is my gripe with buildkite) 🙃

It's particularly weird since I can't reproduce locally

@DilumAluthge
Copy link
Member

DilumAluthge commented Apr 20, 2022

Alright, the minimum functionality of https://github.com/JuliaCI/merge-commit-buildkite-plugin is now there.

Basically, you just add this to the top of your plugin list:

- JuliaCI/merge-commit: ~

So e.g.

steps:
  - plugins:
    - JuliaCI/merge-commit: ~

You'll want to make sure you do this for every step.

Note: eventually, you'll want to replace JuliaCI/merge-commit: ~ with JuliaCI/merge-commit#v1: ~, but that'll have to wait until we first make a tag on the https://github.com/JuliaCI/merge-commit-buildkite-plugin repo.

.buildkite/generator.jl Outdated Show resolved Hide resolved
@DilumAluthge
Copy link
Member

You should run the generator inside Sandbox.jl.

@vchuravy
Copy link
Member Author

You should run the generator inside Sandbox.jl.

How can I tell if I am? The cp error just turned out to be a user error

.buildkite/generator.jl Outdated Show resolved Hide resolved
.buildkite/generator.jl Outdated Show resolved Hide resolved
@DilumAluthge
Copy link
Member

DilumAluthge commented Apr 21, 2022

It looks like we are now checking out the merge commit correctly!

.buildkite/utils.jl Outdated Show resolved Hide resolved
.buildkite/utils.jl Outdated Show resolved Hide resolved
:plugins => plugins(),
:timeout_in_minutes => 30,
:concurrency => 1,
:concurrency_group => "yggdrasil/register",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe do project name here

:label => "register -- $NAME",
:agents => agent(),
:plugins => plugins(),
:timeout_in_minutes => 30,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look at time out

.buildkite/utils.jl Outdated Show resolved Hide resolved
@vchuravy
Copy link
Member Author

vchuravy commented Jan 7, 2023

@giordano sadly NUMA has no warnings, do you have suggestions for a package that is fast and does emit some warnings?

@giordano
Copy link
Member

giordano commented Jan 7, 2023

GR has some hundreds of warnings and it shouldn't take too long: https://dev.azure.com/JuliaPackaging/Yggdrasil/_build/results?buildId=24439&view=results

@vchuravy
Copy link
Member Author

From Elliot:

  1. Generator job that analyzes the diff, and launches pipelines based on that.
  2. Builder jobs that do the actual building. We want these to be skipped if they’re building the same thing as was built previously, so we’ll use coppermind (with cryptic, to store the S3 access key) to skip the actual build step if a treehash match is found
  3. Registration job that downloads the binaries from buildkite artifacts again (since coppermind’s caching on S3 is an implementation detail, not the true storage location), then uploads them to GH releases

.buildkite/NOTES.md Outdated Show resolved Hide resolved
.buildkite/path_processors/per-project Outdated Show resolved Hide resolved
.buildkite/pipeline.yml Show resolved Hide resolved
.buildkite/generator.jl Outdated Show resolved Hide resolved
"inputs" => [
PROJECT,
".ci/",
# ?meta.json
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@staticfloat in AZP we hash in the meta.json Not sure how to do that here

@vchuravy vchuravy marked this pull request as ready for review January 16, 2023 13:12
@vchuravy
Copy link
Member Author

So the last steps are:

Copy link
Member

@giordano giordano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mentioned in the CI call, but repeating it also here: we should implement also here the [skip build] keyword, which doesn't skip the CI job completely, only the build steps:

# Keyword to be used in the commit message to skip a rebuild
SKIP_BUILD_COOKIE="[skip build]"
# This variable will tell us whether we want to skip the build
export SKIP_BUILD="false"
if [[ $(Build.Reason) == "PullRequest" ]]; then
# If we're on a PR though, we look at the entire branch at once
TARGET_BRANCH="remotes/origin/$(System.PullRequest.TargetBranch)"
COMPARE_AGAINST=$(git merge-base --fork-point ${TARGET_BRANCH} HEAD)
git fetch origin "refs/pull/$(System.PullRequest.PullRequestNumber)/head:refs/remotes/origin/pr/$(System.PullRequest.PullRequestNumber)"
if [[ "$(git show -s --format=%B origin/pr/$(System.PullRequest.PullRequestNumber))" == *"${SKIP_BUILD_COOKIE}"* ]]; then
SKIP_BUILD="true"
fi
else
if [[ "$(git show -s --format=%B)" == *"${SKIP_BUILD_COOKIE}"* ]]; then
SKIP_BUILD="true"
fi
fi
And this will be used during registration to re-use the artifacts from the previous build:
skip_build = get(ENV, "SKIP_BUILD", "false") == "true"

@@ -3,6 +3,7 @@ using BinaryBuilder
name = "HelloWorldC"
version = v"1.2.1"


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bump the version.

function download_cached_binaries(download_dir)
NAME = ENV["NAME"]
PROJECT = ENV["PROJECT"]
artifacts = "$(PROJECT)/products/$(NAME)*.tar.gz"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does globbing work in the Cmd below?

@giordano
Copy link
Member

Looking at JuliaBinaryWrappers/HelloWorldC_jll.jl@bd36c4c I believe you aren't exporting the environment variable

YGGDRASIL=true

are you? That's currently set in https://github.com/JuliaPackaging/Yggdrasil/blob/8e7a9c4f1761fcb71c8d2e33688d36ff6fb9f9d2/.ci/azp_agent/.env, so you may have missed it.

Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>
@vchuravy vchuravy merged commit a1b9ad5 into master Jan 17, 2023
@vchuravy vchuravy deleted the bk/initial branch January 17, 2023 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci continuous integration meta 🌳 Issues and pull requested related to Yggdrasil itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants